-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggestions popup #28
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start -- a few suggestions for improvement added.
Please attach a screen recording to the PR each time you make a round of changes.
A gif will work too -- look up LICEcap.
/// it will be slightly below the tapped area. | ||
final newDy = position.dy + 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to position the popup relative to the text, and not tap area?
Hardcoded offset from tap location won't look well if the font size is larger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will have to check that.
…l assertion issue as suggested by Yuri
…displayed where the user tapped.
final Widget Function( | ||
String name, | ||
String message, | ||
Color color, | ||
List<String> replacements, | ||
Function(String) onSuggestionTap, | ||
Function() onClose, | ||
)? mistakeBuilder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- No need to provide the colour ourselves. Let it be resolved dynamically from MistakeType.
- Use typedef for better readability.
final Widget Function( | |
String name, | |
String message, | |
Color color, | |
List<String> replacements, | |
Function(String) onSuggestionTap, | |
Function() onClose, | |
)? mistakeBuilder; | |
typedef ReplacementSuggestionsBuilder = Widget Function( | |
Mistake mistake, | |
List<String> replacements, | |
Function(String) onSuggestionTap, | |
Function() onClose, | |
); | |
final ReplacementSuggestionsBuilder? mistakeBuilder; |
} | ||
|
||
@override | ||
void dispose() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to the bottom of the class.
That's company convention -- all cleanup/disposal should be placed at the very bottom.
This PR contains all the work related to the suggestions popup that appears on a mistake.